-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[lldb] Ignore trailing spaces on quit confirmation #162263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks good but this should come with a (PExpect) test.
@llvm/pr-subscribers-lldb Author: Ebuka Ezike (da-viper) ChangesFull diff: https://github.com/llvm/llvm-project/pull/162263.diff 2 Files Affected:
diff --git a/lldb/source/Core/IOHandler.cpp b/lldb/source/Core/IOHandler.cpp
index 57819eeade6e8..c2530aa0d00c5 100644
--- a/lldb/source/Core/IOHandler.cpp
+++ b/lldb/source/Core/IOHandler.cpp
@@ -152,15 +152,16 @@ void IOHandlerConfirm::IOHandlerComplete(IOHandler &io_handler,
void IOHandlerConfirm::IOHandlerInputComplete(IOHandler &io_handler,
std::string &line) {
- if (line.empty()) {
+ const llvm::StringRef input = llvm::StringRef(line).rtrim();
+ if (input.empty()) {
// User just hit enter, set the response to the default
m_user_response = m_default_response;
io_handler.SetIsDone(true);
return;
}
- if (line.size() == 1) {
- switch (line[0]) {
+ if (input.size() == 1) {
+ switch (input[0]) {
case 'y':
case 'Y':
m_user_response = true;
@@ -176,10 +177,10 @@ void IOHandlerConfirm::IOHandlerInputComplete(IOHandler &io_handler,
}
}
- if (line == "yes" || line == "YES" || line == "Yes") {
+ if (input.equals_insensitive("yes")) {
m_user_response = true;
io_handler.SetIsDone(true);
- } else if (line == "no" || line == "NO" || line == "No") {
+ } else if (input.equals_insensitive("no")) {
m_user_response = false;
io_handler.SetIsDone(true);
}
diff --git a/lldb/test/API/driver/quit_speed/TestQuitWithProcess.py b/lldb/test/API/driver/quit_speed/TestQuitWithProcess.py
index 2412b295bfb59..305e3cc397cf0 100644
--- a/lldb/test/API/driver/quit_speed/TestQuitWithProcess.py
+++ b/lldb/test/API/driver/quit_speed/TestQuitWithProcess.py
@@ -33,3 +33,28 @@ def test_run_quit(self):
child.sendline("quit")
print("sent quit")
child.expect(pexpect.EOF, timeout=15)
+
+ @skipIfAsan
+ def test_run_quit_with_prompt(self):
+ """Test that the lldb driver's batch mode works correctly with trailing space in confimation."""
+ import pexpect
+
+ self.build()
+
+ exe = self.getBuildArtifact("a.out")
+
+ self.launch(executable=exe)
+ child = self.child
+
+ # Launch the process without a TTY so we don't have to interrupt:
+ child.sendline("process launch -n")
+ print("launched process")
+ child.expect(r"Process ([\d]*) launched:")
+ print("Got launch message")
+ child.sendline("quit")
+ print("sent quit")
+
+ child.expect(r".*LLDB will kill one or more processes.*")
+ # add trailing space to the confirmation.
+ child.sendline("yEs ")
+ child.expect(pexpect.EOF, timeout=15)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
No description provided.